Skip to content

GH-48094: [C++] Restrict SecureString capacity tail check to Linux - #49906

Merged
pitrou merged 1 commit into
apache:mainfrom
ArnavBalyan:arnavb/secure-string-msvc-asan
May 5, 2026
Merged

GH-48094: [C++] Restrict SecureString capacity tail check to Linux#49906
pitrou merged 1 commit into
apache:mainfrom
ArnavBalyan:arnavb/secure-string-msvc-asan

Conversation

@ArnavBalyan

@ArnavBalyan ArnavBalyan commented May 1, 2026

Copy link
Copy Markdown
Member

Rationale for this change

What changes are included in this PR?

  • Guard for platform specific behavior.

Are these changes tested?

  • Yes

Are there any user-facing changes?

  • Yes

@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #48094 has been automatically assigned in GitHub to PR creator.

@ArnavBalyan

Copy link
Copy Markdown
Member Author

cc @pitrou thanks

@pitrou

pitrou commented May 5, 2026

Copy link
Copy Markdown
Member

Can we condition this on MSVC rather than Linux? The test can still be used under macOS and other platforms.

@ArnavBalyan
ArnavBalyan force-pushed the arnavb/secure-string-msvc-asan branch from 69d0341 to 1416cc9 Compare May 5, 2026 09:53
@ArnavBalyan
ArnavBalyan force-pushed the arnavb/secure-string-msvc-asan branch from 1416cc9 to 1279689 Compare May 5, 2026 09:56
@ArnavBalyan

Copy link
Copy Markdown
Member Author

done thanks

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, thank you @ArnavBalyan

@pitrou
pitrou merged commit 25e4fdb into apache:main May 5, 2026
54 of 56 checks passed
@pitrou pitrou removed the awaiting review Awaiting review label May 5, 2026
@pitrou

pitrou commented May 5, 2026

Copy link
Copy Markdown
Member

Oops, I should have changed the PR title before merging. Too late.

@github-actions github-actions Bot added the awaiting committer review Awaiting committer review label May 5, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 0 benchmarking runs that have been run so far on merge-commit 25e4fdb.

None of the specified runs were found on the Conbench server.

The full Conbench report has more details.

@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 25e4fdb.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.

Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
…nux (apache#49906)

### Rationale for this change
  - The test reads the unused tail of string past size(), which is undefined. libstdc++ keeps that range addressable, but MSVC does not.
  - Ensure the check is not run for MSVC. Existing assertions still run, the unused capacity is not asserted on non linux platforms.
  - Closes apache#48094.

### What changes are included in this PR?
 - Guard for platform specific behavior.

### Are these changes tested?
 - Yes
### Are there any user-facing changes?
 - Yes
* GitHub Issue: apache#48094

Authored-by: Arnav Balyan <arnavbalyan1@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++] Arrow test 'arrow-utility-test' contains heap-buffer-overflow error

2 participants